Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: make printing output easier (approach 1) #3797

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

I'm looking if we can make printing output easier, and without linters warning about all the unhandled errors for fmt.FPrintXX().

One approach could be to use Cobra's print functions, which are convenient, but the (big!) downside is that we would then have to pass both cobra.Command and command.Cli everywhere.

Looking at our code, it looks like the intent is to separate functionality from cobra (cobra "triggers" the command, but the command itself is agnostic to being called from cobra).

The other approach is to

  • enhance the CLI to have utilities for printing (similar to cobra); dockerCli.Printf(...)
  • enhance the streams package to have utilities, but note that while both dockerCli.Out() and dockerCli.Err() are an io.Writer, only dockerCli.Out() is a streams.Out, so we would have to change dockerCli.Err() to also be a streams.Out. Maybe there was a reason for not doing so (streams.Out can detect if a terminal is attached, maybe that's not possible for STDERR, which (unlike STDOUT is not buffered? not sure if that makes a difference)

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

I'm looking if we can make printing output easier, and without linters warning
about all the unhandled errors for `fmt.FPrintXX()`.

One approach could be to use Cobra's print functions, which are convenient,
but the (big!) downside is that we would then have to pass _both_ `cobra.Command`
_and_ `command.Cli` everywhere.

Looking at our code, it looks like the intent is to separate functionality from
cobra (cobra "triggers" the command, but the command itself is agnostic to being
called from cobra).

The other approach is to

- enhance the CLI to have utilities for printing (similar to cobra); `dockerCli.Printf(...)`
- enhance the `streams` package to have utilities, but note that while both `dockerCli.Out()`
  and `dockerCli.Err()` are an `io.Writer`, only `dockerCli.Out()` is a `streams.Out`, so
  we would have to change `dockerCli.Err()` to also be a `streams.Out`. Maybe there
  was a reason for not doing so (`streams.Out` can detect if a terminal is attached,
  maybe that's not possible for `STDERR`, which (unlike `STDOUT` is not buffered? not
  sure if that makes a difference)

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant